[processor/metricstransform] Fix aggregation of exponential histograms #39143

enisoc · 2025-04-03T15:53:13Z

Description

A panic would occur if the histograms being merged had different numbers of populated entries in the BucketCounts array.

Also the counts for the Zero buckets were not being merged.

Example config:

processors:
  metricstransform:
    transforms:
    - action: combine
      aggregation_type: sum
      include: traces.span.metrics.duration
      new_name: traces.span.metrics.duration

Example panic:

panic: runtime error: index out of range [133] with length 133

goroutine 177 [running]:
go.opentelemetry.io/collector/pdata/pcommon.UInt64Slice.At(...)
	go.opentelemetry.io/collector/[email protected]/pcommon/generated_uint64slice.go:55
github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/aggregateutil.mergeExponentialHistogramDataPoints(0x40014b57e0?, {0x4007bc7220?, 0x400744fee8?})
	github.com/open-telemetry/opentelemetry-collector-contrib/internal/[email protected]/aggregateutil/aggregate.go:309 +0x888
github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/aggregateutil.MergeDataPoints({0x4001a435c0?, 0x400744fee8?}, {0x40011b4078?, 0x60?}, {0x0?, 0x0?, 0x0?, 0x4006a73410?})
	github.com/open-telemetry/opentelemetry-collector-contrib/internal/[email protected]/aggregateutil/aggregate.go:96 +0x208
github.com/open-telemetry/opentelemetry-collector-contrib/processor/metricstransformprocessor.groupMetrics({0x4001b048a0?, 0x400744fed8?}, {0x40011b4078, 0x3}, {0x4001a435c0?, 0x400744fee8?})
	github.com/open-telemetry/opentelemetry-collector-contrib/processor/[email protected]/metrics_transform_processor_otlp.go:451 +0xb8
github.com/open-telemetry/opentelemetry-collector-contrib/processor/metricstransformprocessor.combine({{0xadce910, 0x40018a0fd8}, {0x40011b4066, 0x7}, {0x4000777800, 0x1c}, 0x0, {0x40011b4078, 0x3}, {0x0, ...}, ...}, ...)
	github.com/open-telemetry/opentelemetry-collector-contrib/processor/[email protected]/metrics_transform_processor_otlp.go:438 +0x2c0
github.com/open-telemetry/opentelemetry-collector-contrib/processor/metricstransformprocessor.(*metricsTransformProcessor).processMetrics.func1.1({0x4000c0fd50?, 0x4007038b80?})
	github.com/open-telemetry/opentelemetry-collector-contrib/processor/[email protected]/metrics_transform_processor_otlp.go:258 +0x4e8
go.opentelemetry.io/collector/pdata/pmetric.ScopeMetricsSlice.RemoveIf({0x40041de338?, 0x4007038b80?}, 0x4000e3f5f8)
	go.opentelemetry.io/collector/[email protected]/pmetric/generated_scopemetricsslice.go:111 +0x80
github.com/open-telemetry/opentelemetry-collector-contrib/processor/metricstransformprocessor.(*metricsTransformProcessor).processMetrics.func1({0x40041de300?, 0x4007038b80?})
	github.com/open-telemetry/opentelemetry-collector-contrib/processor/[email protected]/metrics_transform_processor_otlp.go:235 +0x64
go.opentelemetry.io/collector/pdata/pmetric.ResourceMetricsSlice.RemoveIf({0x4000e3c378?, 0x4007038b80?}, 0x4000e3f6e8)
	go.opentelemetry.io/collector/[email protected]/pmetric/generated_resourcemetricsslice.go:111 +0x80
github.com/open-telemetry/opentelemetry-collector-contrib/processor/metricstransformprocessor.(*metricsTransformProcessor).processMetrics(0x4000e3f748?, {0x50ac40c?, 0x10?}, {0x4000e3c378?, 0x4007038b80?})
	github.com/open-telemetry/opentelemetry-collector-contrib/processor/[email protected]/metrics_transform_processor_otlp.go:234 +0x74
go.opentelemetry.io/collector/processor/processorhelper.NewMetrics.func1({0xad9c090, 0x11b475c0}, {0x4000e3c378?, 0x4007038b80?})
	go.opentelemetry.io/collector/[email protected]/processorhelper/metrics.go:55 +0xfc
go.opentelemetry.io/collector/consumer.ConsumeMetricsFunc.ConsumeMetrics(...)
	go.opentelemetry.io/collector/[email protected]/metrics.go:27
go.opentelemetry.io/collector/processor/processorhelper.NewMetrics.func1({0xad9c090, 0x11b475c0}, {0x4000e3c378?, 0x4007038b80?})
	go.opentelemetry.io/collector/[email protected]/processorhelper/metrics.go:66 +0x21c
go.opentelemetry.io/collector/consumer.ConsumeMetricsFunc.ConsumeMetrics(...)
	go.opentelemetry.io/collector/[email protected]/metrics.go:27
go.opentelemetry.io/collector/processor/processorhelper.NewMetrics.func1({0xad9c090, 0x11b475c0}, {0x4000e3c2e8?, 0x4007038390?})
	go.opentelemetry.io/collector/[email protected]/processorhelper/metrics.go:66 +0x21c
go.opentelemetry.io/collector/consumer.ConsumeMetricsFunc.ConsumeMetrics(...)
	go.opentelemetry.io/collector/[email protected]/metrics.go:27
go.opentelemetry.io/collector/processor/processorhelper.NewMetrics.func1({0xad9c090, 0x11b475c0}, {0x4000e3c2e8?, 0x4007038390?})
	go.opentelemetry.io/collector/[email protected]/processorhelper/metrics.go:66 +0x21c
go.opentelemetry.io/collector/consumer.ConsumeMetricsFunc.ConsumeMetrics(...)
	go.opentelemetry.io/collector/[email protected]/metrics.go:27
go.opentelemetry.io/collector/processor/processorhelper.NewMetrics.func1({0xad9c090, 0x11b475c0}, {0x4000e3c2e8?, 0x4007038390?})
	go.opentelemetry.io/collector/[email protected]/processorhelper/metrics.go:66 +0x21c
go.opentelemetry.io/collector/consumer.ConsumeMetricsFunc.ConsumeMetrics(...)
	go.opentelemetry.io/collector/[email protected]/metrics.go:27
go.opentelemetry.io/collector/consumer.ConsumeMetricsFunc.ConsumeMetrics(...)
	go.opentelemetry.io/collector/[email protected]/metrics.go:27
go.opentelemetry.io/collector/internal/fanoutconsumer.(*metricsConsumer).ConsumeMetrics(0x400197bd40, {0xad9c090, 0x11b475c0}, {0x4000e3c2e8?, 0x4007038390?})
	go.opentelemetry.io/collector/internal/[email protected]/metrics.go:60 +0x1ec
github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector.(*connectorImp).exportMetrics(0x4000203a40, {0xad9c090, 0x11b475c0})
	github.com/open-telemetry/opentelemetry-collector-contrib/connector/[email protected]/connector.go:258 +0x110
github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector.(*connectorImp).Start.func1()
	github.com/open-telemetry/opentelemetry-collector-contrib/connector/[email protected]/connector.go:213 +0x48
created by github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector.(*connectorImp).Start in goroutine 1
	github.com/open-telemetry/opentelemetry-collector-contrib/connector/[email protected]/connector.go:207 +0xa8

dehaansa

Overall makes sense to me, but I think there should be a test for each possible case of mismatched bucket count.

dehaansa · 2025-04-03T17:46:49Z

internal/coreinternal/aggregateutil/aggregate.go

+			// Note that groupExponentialHistogramDataPoints() has already ensured that we only try
+			// to merge exponential histograms with matching Scale and Positive/Negative Offsets,
+			// so the corresponding array items in BucketCounts have the same bucket boundaries.
+			// However, the number of buckets may differ depending on what values have been observed.
 			for b := 0; b < dps.At(i).Negative().BucketCounts().Len(); b++ {


I see how this should handle cases where negatives.Len() > dps.At(i).Negatives.BucketCounts().Len(), but I think it should be covered in tests to ensure that future maintenance doesn't break the behavior accidentally.

dehaansa · 2025-04-03T17:48:56Z

It looks like the codeowner allowlist has been fixed on main, so a merge/rebase should resolve that failing check.

A panic would occur if the histograms being merged had different numbers of populated entries in the BucketCounts array. Also the counts for the Zero buckets were not being merged.

For negative buckets, and for a second datapoint with fewer instead of more bucket count array items than the first.

enisoc · 2025-04-03T19:32:07Z

@dehaansa Thanks! I've added some more cases to the test. Is it normal for the CI workflows to take this long, or is there something stuck?

dehaansa · 2025-04-03T20:01:04Z

@dehaansa Thanks! I've added some more cases to the test. Is it normal for the CI workflows to take this long, or is there something stuck?

For folks that aren't yet members of the otel community CI has to be approved before it runs, sorry! I've approved this run, feel free to ping me again if you need additional runs approved.

dehaansa · 2025-04-03T20:24:15Z

@enisoc we'll give the component codeowners some time to respond, and if they don't get back within a couple days we'll go ahead and mark this ready to merge.

open-telemetry#39143) #### Description A panic would occur if the histograms being merged had different numbers of populated entries in the BucketCounts array. Also the counts for the Zero buckets were not being merged. Example config: ``` processors: metricstransform: transforms: - action: combine aggregation_type: sum include: traces.span.metrics.duration new_name: traces.span.metrics.duration ``` Example panic: ``` panic: runtime error: index out of range [133] with length 133 goroutine 177 [running]: go.opentelemetry.io/collector/pdata/pcommon.UInt64Slice.At(...) go.opentelemetry.io/collector/[email protected]/pcommon/generated_uint64slice.go:55 github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/aggregateutil.mergeExponentialHistogramDataPoints(0x40014b57e0?, {0x4007bc7220?, 0x400744fee8?}) github.com/open-telemetry/opentelemetry-collector-contrib/internal/[email protected]/aggregateutil/aggregate.go:309 +0x888 github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/aggregateutil.MergeDataPoints({0x4001a435c0?, 0x400744fee8?}, {0x40011b4078?, 0x60?}, {0x0?, 0x0?, 0x0?, 0x4006a73410?}) github.com/open-telemetry/opentelemetry-collector-contrib/internal/[email protected]/aggregateutil/aggregate.go:96 +0x208 github.com/open-telemetry/opentelemetry-collector-contrib/processor/metricstransformprocessor.groupMetrics({0x4001b048a0?, 0x400744fed8?}, {0x40011b4078, 0x3}, {0x4001a435c0?, 0x400744fee8?}) github.com/open-telemetry/opentelemetry-collector-contrib/processor/[email protected]/metrics_transform_processor_otlp.go:451 +0xb8 github.com/open-telemetry/opentelemetry-collector-contrib/processor/metricstransformprocessor.combine({{0xadce910, 0x40018a0fd8}, {0x40011b4066, 0x7}, {0x4000777800, 0x1c}, 0x0, {0x40011b4078, 0x3}, {0x0, ...}, ...}, ...) github.com/open-telemetry/opentelemetry-collector-contrib/processor/[email protected]/metrics_transform_processor_otlp.go:438 +0x2c0 github.com/open-telemetry/opentelemetry-collector-contrib/processor/metricstransformprocessor.(*metricsTransformProcessor).processMetrics.func1.1({0x4000c0fd50?, 0x4007038b80?}) github.com/open-telemetry/opentelemetry-collector-contrib/processor/[email protected]/metrics_transform_processor_otlp.go:258 +0x4e8 go.opentelemetry.io/collector/pdata/pmetric.ScopeMetricsSlice.RemoveIf({0x40041de338?, 0x4007038b80?}, 0x4000e3f5f8) go.opentelemetry.io/collector/[email protected]/pmetric/generated_scopemetricsslice.go:111 +0x80 github.com/open-telemetry/opentelemetry-collector-contrib/processor/metricstransformprocessor.(*metricsTransformProcessor).processMetrics.func1({0x40041de300?, 0x4007038b80?}) github.com/open-telemetry/opentelemetry-collector-contrib/processor/[email protected]/metrics_transform_processor_otlp.go:235 +0x64 go.opentelemetry.io/collector/pdata/pmetric.ResourceMetricsSlice.RemoveIf({0x4000e3c378?, 0x4007038b80?}, 0x4000e3f6e8) go.opentelemetry.io/collector/[email protected]/pmetric/generated_resourcemetricsslice.go:111 +0x80 github.com/open-telemetry/opentelemetry-collector-contrib/processor/metricstransformprocessor.(*metricsTransformProcessor).processMetrics(0x4000e3f748?, {0x50ac40c?, 0x10?}, {0x4000e3c378?, 0x4007038b80?}) github.com/open-telemetry/opentelemetry-collector-contrib/processor/[email protected]/metrics_transform_processor_otlp.go:234 +0x74 go.opentelemetry.io/collector/processor/processorhelper.NewMetrics.func1({0xad9c090, 0x11b475c0}, {0x4000e3c378?, 0x4007038b80?}) go.opentelemetry.io/collector/[email protected]/processorhelper/metrics.go:55 +0xfc go.opentelemetry.io/collector/consumer.ConsumeMetricsFunc.ConsumeMetrics(...) go.opentelemetry.io/collector/[email protected]/metrics.go:27 go.opentelemetry.io/collector/processor/processorhelper.NewMetrics.func1({0xad9c090, 0x11b475c0}, {0x4000e3c378?, 0x4007038b80?}) go.opentelemetry.io/collector/[email protected]/processorhelper/metrics.go:66 +0x21c go.opentelemetry.io/collector/consumer.ConsumeMetricsFunc.ConsumeMetrics(...) go.opentelemetry.io/collector/[email protected]/metrics.go:27 go.opentelemetry.io/collector/processor/processorhelper.NewMetrics.func1({0xad9c090, 0x11b475c0}, {0x4000e3c2e8?, 0x4007038390?}) go.opentelemetry.io/collector/[email protected]/processorhelper/metrics.go:66 +0x21c go.opentelemetry.io/collector/consumer.ConsumeMetricsFunc.ConsumeMetrics(...) go.opentelemetry.io/collector/[email protected]/metrics.go:27 go.opentelemetry.io/collector/processor/processorhelper.NewMetrics.func1({0xad9c090, 0x11b475c0}, {0x4000e3c2e8?, 0x4007038390?}) go.opentelemetry.io/collector/[email protected]/processorhelper/metrics.go:66 +0x21c go.opentelemetry.io/collector/consumer.ConsumeMetricsFunc.ConsumeMetrics(...) go.opentelemetry.io/collector/[email protected]/metrics.go:27 go.opentelemetry.io/collector/processor/processorhelper.NewMetrics.func1({0xad9c090, 0x11b475c0}, {0x4000e3c2e8?, 0x4007038390?}) go.opentelemetry.io/collector/[email protected]/processorhelper/metrics.go:66 +0x21c go.opentelemetry.io/collector/consumer.ConsumeMetricsFunc.ConsumeMetrics(...) go.opentelemetry.io/collector/[email protected]/metrics.go:27 go.opentelemetry.io/collector/consumer.ConsumeMetricsFunc.ConsumeMetrics(...) go.opentelemetry.io/collector/[email protected]/metrics.go:27 go.opentelemetry.io/collector/internal/fanoutconsumer.(*metricsConsumer).ConsumeMetrics(0x400197bd40, {0xad9c090, 0x11b475c0}, {0x4000e3c2e8?, 0x4007038390?}) go.opentelemetry.io/collector/internal/[email protected]/metrics.go:60 +0x1ec github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector.(*connectorImp).exportMetrics(0x4000203a40, {0xad9c090, 0x11b475c0}) github.com/open-telemetry/opentelemetry-collector-contrib/connector/[email protected]/connector.go:258 +0x110 github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector.(*connectorImp).Start.func1() github.com/open-telemetry/opentelemetry-collector-contrib/connector/[email protected]/connector.go:213 +0x48 created by github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector.(*connectorImp).Start in goroutine 1 github.com/open-telemetry/opentelemetry-collector-contrib/connector/[email protected]/connector.go:207 +0xa8 ``` --------- Co-authored-by: Antoine Toulme <[email protected]>

enisoc requested a review from a team as a code owner April 3, 2025 15:53

enisoc requested a review from dehaansa April 3, 2025 15:53

github-actions bot assigned dehaansa Apr 3, 2025

github-actions bot added the internal/core label Apr 3, 2025

dehaansa approved these changes Apr 3, 2025

View reviewed changes

enisoc added 3 commits April 3, 2025 13:17

[processor/metricstransform] Fix aggregation of exponential histograms

d4c99c6

A panic would occur if the histograms being merged had different numbers of populated entries in the BucketCounts array. Also the counts for the Zero buckets were not being merged.

Add changelog entry

8e64654

Add more test cases

ab4293e

For negative buckets, and for a second datapoint with fewer instead of more bucket count array items than the first.

enisoc force-pushed the aggregateutil-exphist-panic branch from d492dea to ab4293e Compare April 3, 2025 18:17

dehaansa added processor/metricstransform Metrics Transform processor waiting-for-code-owners labels Apr 3, 2025

github-actions bot requested a review from dmitryax April 3, 2025 20:22

Merge branch 'main' into aggregateutil-exphist-panic

d3da368

dehaansa added the ready to merge Code review completed; ready to merge by maintainers label Apr 6, 2025

songy23 merged commit ab74619 into open-telemetry:main Apr 7, 2025
179 checks passed

github-actions bot added this to the next release milestone Apr 7, 2025

enisoc deleted the aggregateutil-exphist-panic branch April 7, 2025 15:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[processor/metricstransform] Fix aggregation of exponential histograms #39143

[processor/metricstransform] Fix aggregation of exponential histograms #39143

Uh oh!

enisoc commented Apr 3, 2025

Uh oh!

dehaansa left a comment

Uh oh!

dehaansa Apr 3, 2025

Uh oh!

dehaansa commented Apr 3, 2025

Uh oh!

enisoc commented Apr 3, 2025

Uh oh!

dehaansa commented Apr 3, 2025 •

edited

Loading

Uh oh!

dehaansa commented Apr 3, 2025

Uh oh!

Uh oh!

Uh oh!

[processor/metricstransform] Fix aggregation of exponential histograms #39143

[processor/metricstransform] Fix aggregation of exponential histograms #39143

Uh oh!

Conversation

enisoc commented Apr 3, 2025

Description

Uh oh!

dehaansa left a comment

Choose a reason for hiding this comment

Uh oh!

dehaansa Apr 3, 2025

Choose a reason for hiding this comment

Uh oh!

dehaansa commented Apr 3, 2025

Uh oh!

enisoc commented Apr 3, 2025

Uh oh!

dehaansa commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dehaansa commented Apr 3, 2025

Uh oh!

Uh oh!

Uh oh!

dehaansa commented Apr 3, 2025 •

edited

Loading